-
Notifications
You must be signed in to change notification settings - Fork 5.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Inconsistent use of v3 and v4 Weaviate clients #13719
base: main
Are you sure you want to change the base?
fix: Inconsistent use of v3 and v4 Weaviate clients #13719
Conversation
All references to Weaviate V3 clients (weaviate.Client) have been changed to Weaviate V4 clients (weaviate.WeaviateClient). Documentation has also been modified to reflect changes. Additionally, the `auth_config` argument to `WeaviateVectorStore.from_params` has been made optional to match the underlying `weaviate.WeaviateClient` signature.
Requires review of existing documentation related to Weaviate integration and potential modification of examples to use V4 Weaviate clients. |
Is this PR planned to be merged ? It has been five months. |
if not isinstance(client, weaviate.WeaviateClient): | ||
raise ValueError( | ||
f"Invalid client type, expected weaviate.WeaviateClient, got {type(client)}" | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is slightly out of date -- you can technically pass in an async client or sync client these days. This would break that
Description
This PR addresses #13614 which relates to inconsistencies between Weaviate V3 and V4 clients in the
llama-index-vector-stores-weaviate
implementation. This causes bugs due to breaking API changes between V3 and V4.All references to Weaviate V3 clients (
weaviate.Client
) have been changed to Weaviate V4 clients (weaviate.WeaviateClient
). The current implementation expects Weaviate V4 clients. Documentation has also been modified to reflect changes.Additionally, the
auth_config
argument toWeaviateVectorStore.from_params
has been made optional to match the underlyingweaviate.WeaviateClient
signature.Fixes #13614
Version Bump?
Did I bump the version in the
pyproject.toml
file of the package I am updating? (Except for thellama-index-core
package)Type of Change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
I manually tested the code against a locally running Weaviate database instance. Specifically, I have run the updated example code in the
WeaviateVectorStore
docstring (changing the Weaviate url as required); and I have manually tested the modifiedWeaviateVectorStore.from_params
method with the same url. Due to the fact that adequately testing the code requires Weaviate infrastructure to test against, and no pattern is currently in place for such integration tests, I have not attempted to add such integration tests in this set of changes.Suggested Checklist:
make format; make lint
to appease the lint gods